mctpd: add dbus method RegisterVDMTypeSupport#132
mctpd: add dbus method RegisterVDMTypeSupport#132jk-ozlabs merged 1 commit intoCodeConstruct:mainfrom
Conversation
msnidhin
force-pushed
the
vdm_msg_type_support
branch
2 times, most recently
from
0d4a51d to
f65eed4
Compare
jk-ozlabs
left a comment
jk-ozlabs
left a comment
There was a problem hiding this comment.
Looks pretty good, thanks!
I think your decision for using a variant in the registration makes sense, we should just ensure that we get an appropriate dbus error returned when an incorrect variant type is used.
A few comments inline, but they're all fairly minor. The test cases may warrant splitting up though.
msnidhin
force-pushed
the
vdm_msg_type_support
branch
2 times, most recently
from
345b4a9 to
0648bd1
Compare
|
New commit looks good, but can you squash that single fix into the main change? |
msnidhin
force-pushed
the
vdm_msg_type_support
branch
from
99b77f1 to
317225a
Compare
Sure. Rebased the branch |
|
Merged, thanks! |
Add a new DBus method RegisterVDMTypeSupport to support vendor defined message support as responder Add a new control command handler for get vdm message Tested: Unit tests passed Tested by sending GetVDM command from another endpoint Signed-off-by: Nidhin MS <nidhin.ms@intel.com> Signed-off-by: PavanKumarIntel <pavanx.kumar.martha@intel.com>
|
I guess one follow-up question though: we should probably report 7e / 7f in the Supported Types when one VDM type has been registered, right? |
|
(the spec doesn't seem to provide any specific guidance here) |
Agree.. that sounds good. Should I open another PR? |
|
Yep, best as an separate PR for that. |
|
Hi @jk-ozlabs |
|
We don't have one in place at the moment, but I'm mainly going for 80-column lines, to match the rest of the codebase. |
3 participants
Add a new DBus method RegisterVDMTypeSupport to support vendor defined message support as responder
Add a new control command handler for get vdm message
Tested:
Unit tests passed
Tested by sending GetVDM command from another endpoint